-
Notifications
You must be signed in to change notification settings - Fork 5
🤖 Add pluggable runtime abstraction layer #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2ae9b82 to
0874776
Compare
✅ Runtime Integration Tests AddedAdded comprehensive integration tests for both Test InfrastructureDocker SSH Server:
Test Matrix Pattern:
Test CoverageCore Operations: (26 tests)
Edge Cases: (26 tests)
ResultsTest run: TEST_INTEGRATION=1 bun x jest tests/runtime/runtime.test.ts
# ~25 seconds for all 52 testsImplementation NotesLocalRuntime fixes:
SSHRuntime enhancements:
Key design decisions:
Files AddedReady for review! 🚀 |
|
Added macOS runtime integration tests to CI. The main integration suite covers Linux, but runtime code is particularly prone to system incompatibilities (process spawning, streams, file ops) so we run these tests on macOS as well. Full suite on matrix would be wasteful - this targets just the platform-sensitive code. |
fd95d45 to
20a2bad
Compare
|
Updated macOS runtime test job:
|
38c283c to
76002b0
Compare
Implements Phase 1 of pluggable runtime system with minimal Runtime interface that allows tools to execute in different environments (local, docker, ssh). Changes: - Add Runtime interface with 5 core methods: exec, readFile, writeFile, stat, exists - Implement LocalRuntime using Node.js APIs (spawn, fs/promises) - Refactor file tools (file_read, file_edit_*) to use runtime abstraction - Update ToolConfiguration to include runtime field - Inject LocalRuntime in aiService and ipcMain - Update tsconfig to ES2023 for Disposable type support - Update all tests to inject LocalRuntime (90 tests pass) This is a pure refactoring with zero user-facing changes. All existing functionality remains identical. Sets foundation for Docker and SSH runtimes. _Generated with `cmux`_
- Add SSHRuntime class implementing Runtime interface - Add runtime configuration types (local, ssh) - Add runtime factory to create runtime based on config - Use native ssh2 SFTP for file operations - Support SSH key and password authentication - Connection pooling and automatic reconnection
- Add runtimeConfig to WorkspaceMetadata - Update AIService to use runtime factory with workspace config - Update all tests and ipcMain to use runtime factory - Default to local runtime if no config specified
- Use async fs.readFile instead of sync readFileSync - Remove async from close() method (no await needed) - Fix any type usage in runtime factory error message
electron-builder tries to run 'rebuild' for native modules, but ssh2 doesn't have native dependencies that need rebuilding. Add a no-op script to satisfy electron-builder.
- Create src/constants/env.ts with NON_INTERACTIVE_ENV_VARS - Update LocalRuntime and bash tool to use shared constant - Eliminates duplicate environment variable definitions
Per review feedback, the Runtime interface should be minimal. The exists() method can be implemented as a utility function using stat(). Changes: - Remove exists() from Runtime interface - Remove implementations from LocalRuntime and SSHRuntime - Create fileExists() utility in src/utils/runtime/fileExists.ts - Update file_edit_insert.ts to use the utility function This keeps the Runtime interface minimal while providing the same functionality through a shared utility.
Benefits:
- Leverages user's SSH config (~/.ssh/config)
- Supports SSH features: config aliases, ProxyJump, ControlMaster, etc.
- No password prompts (assumes key-based auth or ssh-agent)
- Simpler configuration (just host + workdir)
- No native dependencies to manage
Changes:
- Removed ssh2 and @types/ssh2 dependencies
- SSHRuntime now uses spawn('ssh') for all operations
- File operations use cat/chmod/mv for atomic writes
- stat() uses 'stat -c' format string for portable output
- Updated RuntimeConfig to remove user, port, keyPath, password
- Config now accepts SSH config aliases (e.g., 'my-server')
Implementation:
- exec(): ssh -T <host> '<command>'
- readFile(): ssh <host> 'cat <path>'
- writeFile(): ssh <host> 'cat > temp && chmod 600 temp && mv temp path'
- stat(): ssh <host> 'stat -c "%s %Y %F" <path>'
Per user request, convert Runtime API to use streaming primitives while providing convenience helpers for existing code patterns. Changes: - Runtime interface now returns Web Streams for all I/O operations - exec() returns ExecStream with stdin/stdout/stderr streams + promises - readFile() returns ReadableStream<Uint8Array> - writeFile() returns WritableStream<Uint8Array> - Added design principle comments: keep Runtime minimal, use helpers Convenience helpers (src/utils/runtime/helpers.ts): - execBuffered(): wraps streaming exec with buffered string output - readFileString(): read file as UTF-8 string - writeFileString(): write string to file atomically Implementation: - LocalRuntime uses Readable.toWeb() / Writable.toWeb() for conversion - SSHRuntime wraps ssh process streams - Both use atomic writes (temp file + rename) - All file tools updated to use helpers Benefits: - Memory-efficient for large files/outputs (streaming) - Simple string-based API via helpers (backward compat) - Foundation ready for Docker runtime streaming logs All tests pass (739 pass, 1 skip)
- Remove FileStat.isFile field (can be inferred from isDirectory) - Update all usages to check isDirectory instead - Update test fixtures to remove isFile - Update error messages to be more descriptive Note: DisposableProcess class was already removed in the streaming conversion - it's no longer needed since exec() directly returns streams without using the 'using' statement pattern.
Add comprehensive integration tests for LocalRuntime and SSHRuntime using a test matrix pattern. All 52 tests run against both implementations to ensure consistent behavior. **Test Infrastructure:** - Docker-based SSH server (Alpine + OpenSSH) - Dynamic port allocation (no hardcoded ports) - Ephemeral SSH key generation per test run - Concurrent test run isolation **Test Coverage:** - Core operations: exec(), readFile(), writeFile(), stat() - Edge cases: non-existent files, directories, binary data, large files - Special cases: concurrent ops, special chars, nested paths - Runtime-specific features (SSH auth, streaming) **Key Features:** - Real SSH testing (no mocking) for production confidence - Dynamic ports support parallel test runs on same machine - Container reused across test suite for speed (~25s total) - Test helpers with disposable workspaces **Changes:** - Add tests/runtime/runtime.test.ts (409 lines, 52 tests) - Add tests/runtime/ssh-fixture.ts (Docker lifecycle) - Add tests/runtime/test-helpers.ts (workspace management) - Add tests/runtime/ssh-server/ (Docker config) - Add SSHRuntime config: identityFile, port options - Fix SSHRuntime: create parent dirs on writeFile - Fix LocalRuntime/SSHRuntime: type annotations for streams All tests passing (52/52). Existing integration tests still pass. _Generated with `cmux`_
Make the timeout field in ExecOptions required (was optional). This ensures all exec() calls have an upper bound on execution time, preventing zombie processes from accumulating. **Changes:** - ExecOptions.timeout: optional -> required (with zombie prevention comment) - Update all call sites in SSHRuntime (readFile: 300s, writeFile: 300s, stat: 10s) - Update all test call sites to provide timeout (30s for fast ops, 60s for cleanup) Rationale: Even long-running commands should have a reasonable upper bound (e.g., 3600s for 1 hour). Without timeouts, processes can leak and exhaust PIDs over long sessions. _Generated with `cmux`_
Add a dedicated CI job to run runtime integration tests on macOS. The main integration suite already covers Linux runtime tests, but we run runtime tests on macOS specifically because this code is particularly prone to system incompatibilities (process spawning, stream handling, file operations). Running the full integration suite on a matrix would be wasteful - we only need to verify runtime behavior across platforms. Changes: - Run all of tests/runtime/ for future-proofing - No API keys needed (runtime tests don't use AI) _Generated with `cmux`_
depot macOS runners include Docker, which is required for SSH runtime tests. Standard macos-latest runners don't have Docker pre-installed.
depot-macos runners don't come with Docker pre-installed. Use douglascamata/setup-docker-macos-action to install Colima + Docker.
Docker setup on macOS runners is problematic: - standard macos-latest runners don't have Docker - depot-macos-15 runs on ARM64 which isn't supported by docker setup actions - Installing Docker on macOS adds ~2-3 minutes overhead via Colima The main integration test suite on Linux already covers runtime tests. LocalRuntime and SSHRuntime are platform-agnostic by design.
Both LocalRuntime and SSHRuntime now require a workdir parameter:
- LocalRuntime(workdir: string)
- SSHRuntime({ host, workdir, ... })
Benefits:
- Symmetric interface - both runtimes bound to a workspace directory
- Less error-prone - no need to pass cwd to every exec() call
- Default cwd fallback - exec() uses workdir if cwd not specified
- Better abstraction - Runtime represents execution environment for a workspace
Updated:
- RuntimeConfig type to require workdir for local
- All call sites in src/ to provide workdir
- All tests to provide workdir
- Add git to SSH test container Dockerfile (enables git operations testing) - Add 12 new test cases covering: - Git operations: init, commit, branches, status - Shell behavior: multi-line output, pipes, command substitution, large output - Error handling: command not found, syntax errors, permission denied - All tests run for both LocalRuntime and SSHRuntime (76 total: 38 × 2) - Tests verify runtime abstraction works consistently across environments Test results: 76 passed (38 local + 38 SSH)
f7b254d to
6aef9cc
Compare
Connects the init hooks system (PR #228) with the Runtime abstraction so workspace creation progress and init hook output stream to the frontend. **Init Hook Utilities (src/runtime/initHook.ts):** - checkInitHookExists(): Check if .cmux/init is executable - getInitHookPath(): Get init hook path for project - LineBuffer class: Line-buffered streaming (handles incomplete lines) - createLineBufferedLoggers(): Creates stdout/stderr line buffers **Runtime Integration:** - InitLogger interface: logStep(), logStdout(), logStderr(), logComplete() - WorkspaceCreationParams extended with initLogger - LocalRuntime: Runs init hook locally via bash, streams output - SSHRuntime: Runs init hook on remote host, streams via Web Streams **IPC Bridge:** - IpcMain creates InitLogger that bridges to InitStateManager - Runtime owns workspace creation entirely (no IPC branching) - Creation steps logged: "Creating worktree...", "Running init hook..." - Real-time streaming to frontend via existing init channels **Testing:** - 7 unit tests for LineBuffer and createLineBufferedLoggers - Integration tests updated with mockInitLogger - All 770 tests passing Generated with `cmux`
- Added InitLogger, WorkspaceCreationParams, WorkspaceCreationResult interfaces to Runtime.ts - Implemented LocalRuntime.createWorkspace() with git worktree creation and init hook support - Added stub SSHRuntime.createWorkspace() (returns not implemented error) - Extracted workspace-creation.test.ts from commit history (370 lines) - Init hook utilities already present from previous commit (initHook.ts + tests) - Updated imports across runtime implementations Restored lost work from reflog commit f7f18dd and related commits.
6c2c03b to
920be74
Compare
Overview
Implements Phase 1 of pluggable runtime system with minimal Runtime interface that allows tools to execute in different environments (local, docker, ssh). This is a pure refactoring with zero user-facing changes.
Changes
Runtime Abstraction
exec(),readFile(),writeFile(),stat(),exists()Tool Refactoring
file_read.tsto useruntime.readFile()andruntime.stat()file_edit_replace.tsto use runtime for all file operationsfile_edit_insert.tsto use runtime for all file operationsfileCommon.tsto work withFileStatinterface instead offs.Statsbash.tskept unchanged (complex overflow handling)Integration
ToolConfigurationto includeruntime: RuntimefieldLocalRuntimein aiService and ipcMainTesting
Architecture Benefits
Easy to test: Runtime implementations are isolated (< 250 lines each), no dependencies between runtimes.
Easy to extend: Adding DockerRuntime or SSHRuntime = implement 5-method interface. Tools automatically work with new runtimes.
Backwards compatible: LocalRuntime is default, no config changes = no behavior changes.
Next Steps
Foundation is now in place for:
runtimeConfigto WorkspaceMetadataTesting
Generated with
cmux